Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

internal/pkg/scorecard: access CRManifestOpt correctly with --olm-deployed #1565

Merged

Conversation

estroz
Copy link
Member

@estroz estroz commented Jun 17, 2019

Description of the change: use viper.GetStringSlice() for getting and use []string when setting CRManifestOpt with --olm-deployed.

Motivation for the change: CRManifestOpt is not accessed correctly when --olm-deployed is set.

…etting and use []string when setting CRManifestOpt
@estroz estroz added kind/bug Categorizes issue or PR as related to a bug. scorecard Issue relates to the scorecard subcomponent labels Jun 17, 2019
@estroz estroz changed the title internal/pkg/scorecard/scorecard.go: access CRManifestOpt correctly with --olm-deployed internal/pkg/scorecard: access CRManifestOpt correctly with --olm-deployed Jun 17, 2019
@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 17, 2019
Copy link
Contributor

@hasbro17 hasbro17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Add a line in the CHANGELOG for this fix.

@estroz estroz requested a review from joelanford June 18, 2019 20:42
Copy link
Member

@joelanford joelanford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

if err != nil {
return nil, err
}
crFile, err := ioutil.TempFile("", "*.cr.yaml")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the file *.cr.yaml now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The temp file names were in an annoying format before 😁. This format uses .cr.yaml as a file extension.

@estroz estroz requested a review from AlexNPavel June 19, 2019 18:35
Copy link
Contributor

@AlexNPavel AlexNPavel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@estroz estroz merged commit c8ead55 into operator-framework:master Jun 19, 2019
@estroz estroz deleted the scorecard-consistent-cr-access branch June 19, 2019 18:41
estroz pushed a commit to estroz/operator-sdk that referenced this pull request Jul 19, 2019
…deployed` (operator-framework#1565)

* internal/pkg/scorecard/scorecard.go: use viper.GetStringSlice() for getting and use []string when setting CRManifestOpt

* CHANGELOG.md: update with scorecard cr-manifests bug fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. scorecard Issue relates to the scorecard subcomponent size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants